Skip to content

Add dependency to gflags and related tests #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Dec 14, 2016
Merged

Add dependency to gflags and related tests #845

merged 6 commits into from
Dec 14, 2016

Conversation

wangkuiyi
Copy link
Collaborator

@wangkuiyi wangkuiyi commented Dec 12, 2016 •
edited
Loading

Fixes #822

@wangkuiyi
Copy link
Collaborator Author

wangkuiyi commented Dec 13, 2016 •
edited
Loading

The current problem is that the building of glog errors:

ERROR: /paddle/third_party/glog_test/BUILD:3:1: Linking of rule '//third_party/glog_test:glog_test' failed: process-wrapper failed: error executing command /root/.cache/bazel/_bazel_root/8c87e0642c6bf396d03eecdb205876e7/execroot/paddle/_bin/process-wrapper -1 5 - - /usr/bin/gcc -o bazel-out/local-fastbuild/bin/third_party/glog_test/glog_test ... (remaining 11 argument(s) skipped).
bazel-out/local-fastbuild/bin/_solib_k8/libexternal_Sglog_Slibglog.so: undefined reference to `google::FlagRegisterer::FlagRegisterer(char const*, char const*, char const*, char const*, void*, void*)'

The problem is that the symbol FlagRegisterer was defined in namespace gflags other than google as glog expected:

root@0c34bd561238:/paddle# nm -gC bazel-out/local-fastbuild/bin/_solib_k8/libexternal_Sgflags_Slibgflags.so | grep -i FlagRegisterer
0000000000024930 W gflags::FlagRegisterer::FlagRegisterer<std::string>(char const*, char const*, char const*, std::string*, std::string*)
0000000000025180 W gflags::FlagRegisterer::FlagRegisterer<bool>(char const*, char const*, char const*, bool*, bool*)
000000000002545a W gflags::FlagRegisterer::FlagRegisterer<double>(char const*, char const*, char const*, double*, double*)
0000000000025212 W gflags::FlagRegisterer::FlagRegisterer<int>(char const*, char const*, char const*, int*, int*)
00000000000252a4 W gflags::FlagRegisterer::FlagRegisterer<unsigned int>(char const*, char const*, char const*, unsigned int*, unsigned int*)
0000000000025336 W gflags::FlagRegisterer::FlagRegisterer<long>(char const*, char const*, char const*, long*, long*)
00000000000253c8 W gflags::FlagRegisterer::FlagRegisterer<unsigned long>(char const*, char const*, char const*, unsigned long*, unsigned long*)
0000000000024930 W gflags::FlagRegisterer::FlagRegisterer<std::string>(char const*, char const*, char const*, std::string*, std::string*)
0000000000025180 W gflags::FlagRegisterer::FlagRegisterer<bool>(char const*, char const*, char const*, bool*, bool*)
000000000002545a W gflags::FlagRegisterer::FlagRegisterer<double>(char const*, char const*, char const*, double*, double*)
0000000000025212 W gflags::FlagRegisterer::FlagRegisterer<int>(char const*, char const*, char const*, int*, int*)
00000000000252a4 W gflags::FlagRegisterer::FlagRegisterer<unsigned int>(char const*, char const*, char const*, unsigned int*, unsigned int*)
0000000000025336 W gflags::FlagRegisterer::FlagRegisterer<long>(char const*, char const*, char const*, long*, long*)
00000000000253c8 W gflags::FlagRegisterer::FlagRegisterer<unsigned long>(char const*, char const*, char const*, unsigned long*, unsigned long*)

The root of the reason is that the BUILD file in gflags repo uses gflags as the default C++ namespace name https://github.com/gflags/gflags/blob/v2.2.0/BUILD#L10. My fix is to provide my own gflags.BUILD which uses google as the default namespace.

reyoung
reyoung approved these changes Dec 14, 2016
View reviewed changes
Copy link
Collaborator

@reyoung reyoung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

reyoung
reyoung reviewed Dec 14, 2016
View reviewed changes
paddle/cuda/src/hl_cuda_device.cc
#include <sys/time.h>
#include <unistd.h>
#include <mutex>
#include "hl_cuda.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件目前还有点问题，参考这个PR #873

所以，麻烦把这个文件revert吧。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我一revert就过不了pre-commit check了。

这里把 #include "hl_cuda.h" 这行挪一下位置应该不会影响编译才对？

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不影响编译，影响了clang-format的自动化format。。

因为似乎clang-format 4.X 和3.8+在这个细节上有区别。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要不先等#873 merge就好了。。手动把这段format关掉就行了。。

或者在您这个PR里面手动关掉。我把我那个PR close掉也行

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@wangkuiyi wangkuiyi merged commit 5ac16e5 into PaddlePaddle:develop Dec 14, 2016
@wangkuiyi wangkuiyi deleted the glog-gflags-bazel branch December 18, 2016 17:01
zhhsplendid pushed a commit to zhhsplendid/Paddle that referenced this pull request Sep 25, 2019
@shanyi15
routine_update_190506 (PaddlePaddle#845)
89dd367
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
Merge pull request PaddlePaddle#845 from leeyy2020/fix_pet_bug
058139e 
Fix pet bug
lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024
@juncaipeng
Add PTQ CE Test (PaddlePaddle#845)
3babb01 
* Add ce test for ptq
zhangyuqin1998 pushed a commit to zhangyuqin1998/Paddle that referenced this pull request Feb 20, 2025
Reduce versbosity in manifest.py (PaddlePaddle#845)
a68e2f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reyoung reyoung reyoung approved these changes

Assignees

@reyoung reyoung

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wangkuiyi @reyoung